-
Notifications
You must be signed in to change notification settings - Fork 8
refactor: 변경된 멘토/멘티 탭 명세에 맞게 api 수정 #422
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: 변경된 멘토/멘티 탭 명세에 맞게 api 수정 #422
Conversation
|
Warning Rate limit exceeded@nayonsoso has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 16 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (11)
Walkthrough
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40–60 minutes
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
- 추가로 N+1 해결
- 추가로, 멘토링에 mentor_id와 mentee_id가 unique 해야 할 것을 염두에 두고, 테스트 코드 수정 - 테스트 단위가 작아지도록 수정
67ce1d3 to
f94d8c3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/test/java/com/example/solidconnection/mentor/service/MentoringQueryServiceTest.java (1)
57-57:mentorUser3변수 선언 일관성 확인 필요
mentorUser3만 로컬 변수로 선언되어 있는데, 다른 사용자들처럼 인스턴스 필드로 선언하는 것이 일관성 있을 것 같습니다. 테스트에서 사용하지 않는다면 생성하지 않는 것도 고려해보세요.- SiteUser mentorUser3 = siteUserFixture.멘토(3, "mentor3"); + SiteUser mentorUser3 = siteUserFixture.멘토(3, "mentor3");그리고 필드 선언부에 추가:
- private SiteUser mentorUser1, mentorUser2; + private SiteUser mentorUser1, mentorUser2, mentorUser3;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
src/main/java/com/example/solidconnection/mentor/controller/MentoringForMenteeController.java(1 hunks)src/main/java/com/example/solidconnection/mentor/controller/MentoringForMentorController.java(3 hunks)src/main/java/com/example/solidconnection/mentor/domain/Mentoring.java(2 hunks)src/main/java/com/example/solidconnection/mentor/dto/CheckMentoringRequest.java(1 hunks)src/main/java/com/example/solidconnection/mentor/dto/CheckedMentoringsResponse.java(1 hunks)src/main/java/com/example/solidconnection/mentor/dto/MentoringForMenteeResponse.java(1 hunks)src/main/java/com/example/solidconnection/mentor/dto/MentoringForMentorResponse.java(1 hunks)src/main/java/com/example/solidconnection/mentor/dto/MentoringListResponse.java(0 hunks)src/main/java/com/example/solidconnection/mentor/repository/MentoringRepository.java(1 hunks)src/main/java/com/example/solidconnection/mentor/service/MentoringCheckService.java(1 hunks)src/main/java/com/example/solidconnection/mentor/service/MentoringCommandService.java(0 hunks)src/main/java/com/example/solidconnection/mentor/service/MentoringQueryService.java(2 hunks)src/main/resources/db/migration/V27__add_checked_at_by_mentee_column.sql(1 hunks)src/test/java/com/example/solidconnection/mentor/fixture/MentoringFixture.java(2 hunks)src/test/java/com/example/solidconnection/mentor/fixture/MentoringFixtureBuilder.java(3 hunks)src/test/java/com/example/solidconnection/mentor/service/MentoringCheckServiceTest.java(1 hunks)src/test/java/com/example/solidconnection/mentor/service/MentoringCommandServiceTest.java(2 hunks)src/test/java/com/example/solidconnection/mentor/service/MentoringQueryServiceTest.java(3 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/com/example/solidconnection/mentor/service/MentoringCommandService.java
- src/main/java/com/example/solidconnection/mentor/dto/MentoringListResponse.java
🚧 Files skipped from review as they are similar to previous changes (15)
- src/main/java/com/example/solidconnection/mentor/dto/CheckMentoringRequest.java
- src/main/resources/db/migration/V27__add_checked_at_by_mentee_column.sql
- src/test/java/com/example/solidconnection/mentor/fixture/MentoringFixture.java
- src/main/java/com/example/solidconnection/mentor/domain/Mentoring.java
- src/main/java/com/example/solidconnection/mentor/dto/CheckedMentoringsResponse.java
- src/main/java/com/example/solidconnection/mentor/service/MentoringCheckService.java
- src/test/java/com/example/solidconnection/mentor/service/MentoringCheckServiceTest.java
- src/main/java/com/example/solidconnection/mentor/dto/MentoringForMenteeResponse.java
- src/test/java/com/example/solidconnection/mentor/service/MentoringCommandServiceTest.java
- src/main/java/com/example/solidconnection/mentor/dto/MentoringForMentorResponse.java
- src/main/java/com/example/solidconnection/mentor/service/MentoringQueryService.java
- src/main/java/com/example/solidconnection/mentor/repository/MentoringRepository.java
- src/test/java/com/example/solidconnection/mentor/fixture/MentoringFixtureBuilder.java
- src/main/java/com/example/solidconnection/mentor/controller/MentoringForMenteeController.java
- src/main/java/com/example/solidconnection/mentor/controller/MentoringForMentorController.java
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: nayonsoso
PR: solid-connection/solid-connect-server#375
File: src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java:47-53
Timestamp: 2025-07-05T17:54:42.475Z
Learning: MentorMyPageService에서 PUT 메서드 구현 시 전체 채널을 새로 생성하여 교체하는 방식을 사용하는 것이 PUT의 의미론적 특성과 일치하며, 트랜잭션 로킹 관점에서도 합리적인 접근이다.
Learnt from: whqtker
PR: solid-connection/solid-connect-server#362
File: src/main/java/com/example/solidconnection/mentor/service/MentoringQueryService.java:0-0
Timestamp: 2025-07-04T10:41:32.999Z
Learning: 멘토링 관련 기능에는 페이지네이션을 적용하지 않는 것이 해당 프로젝트의 설계 방침이다.
📚 Learning: 멘토링 조회 기능에는 페이지네이션을 적용하지 않는 것이 요구사항입니다. 멘토링 데이터의 특성상 대량 데이터 처리가 필요하지 않을 것으로 예상됩니다....
Learnt from: whqtker
PR: solid-connection/solid-connect-server#362
File: src/main/java/com/example/solidconnection/mentor/service/MentoringQueryService.java:0-0
Timestamp: 2025-07-04T10:41:20.575Z
Learning: 멘토링 조회 기능에는 페이지네이션을 적용하지 않는 것이 요구사항입니다. 멘토링 데이터의 특성상 대량 데이터 처리가 필요하지 않을 것으로 예상됩니다.
Applied to files:
src/test/java/com/example/solidconnection/mentor/service/MentoringQueryServiceTest.java
🧬 Code Graph Analysis (1)
src/test/java/com/example/solidconnection/mentor/service/MentoringQueryServiceTest.java (2)
src/test/java/com/example/solidconnection/mentor/service/MentoringCommandServiceTest.java (2)
Nested(70-90)Nested(92-175)src/test/java/com/example/solidconnection/mentor/service/MentoringCheckServiceTest.java (3)
Nested(64-101)Nested(103-170)Nested(141-169)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
src/test/java/com/example/solidconnection/mentor/service/MentoringQueryServiceTest.java (4)
4-27: 임포트 추가가 적절합니다!새로운 페이지네이션 기능과 역할별 응답 DTO를 지원하기 위한 필수 임포트들이 잘 추가되었네요.
68-132: 멘토 테스트가 명세 변경사항을 잘 반영합니다!변경된 API 명세에 따라:
- 모든 상태의 멘토링을 조회하는 테스트
- 상대방(멘티) 정보 포함 여부 확인
- 멘토의 확인 여부(isChecked) 검증
- 빈 리스트 처리
모든 케이스가 잘 커버되어 있습니다.
135-234: 멘티 테스트가 역할별 요구사항을 잘 구현했습니다!변경된 명세에 따른 주요 검증 사항:
- 거절된 멘토링 조회 시 예외 발생 ✓
- 상태별(승인/대기) 필터링 ✓
- 상대방(멘토) 정보 포함 ✓
- 멘티의 확인 여부(isChecked) 검증 ✓
- 빈 리스트 처리 ✓
모든 시나리오가 적절히 테스트되고 있습니다.
26-27: 페이지네이션 적용이 프로젝트 방침과 일치하는지 확인 필요이전 학습 내용에 따르면 멘토링 기능에는 페이지네이션을 적용하지 않는 것이 프로젝트 방침이었습니다. 하지만 이번 리팩토링에서
Pageable과SliceResponse를 사용하고 있네요.명세 변경으로 인한 의도적인 변경인지 확인이 필요합니다.
Also applies to: 51-51, 64-64
f94d8c3 to
09ee5b3
Compare
whqtker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
일단 approve하고 천천히 보겠습니다 !
관련 이슈
작업 내용
커밋을 따라 읽어주세요~
PR분량이 많고, 순도 100%의 코드리뷰도 하지 않으므로 이번에는 PR 디스크립션을 꼼꼼히 작성해보겠습니다🫡
MentoringCheckService,MentoringCommandService,MentoringQueryService이렇게 세가지가 되었습니다. 이 지점에서 "🤔컨트롤러는 역할별로 나눴으면서, 서비스는 기능별로 나누었네?" 라는 의문이 드실 수 있으실텐데요. 지금의 프로젝트에서는 권한 관련 어노테이션이 컨트롤러에 있기 때문에 컨트로러는 권한별로 나누어도 괜찮지만, 서비스 코드는 기능별로 관리하는게 응집도도 있고, 공통되는 코드를 관리하기도 쉬울것이라고 생각했습니다. (e.g. check 에 대해서, 멘토와 멘티 서비스에 각각 있다면 private 함수를 재사용하지 못할 것)특이 사항
오늘(토요일)까지 머지하겠습니다. 🏃♀️🏃♀️